-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
relation summary #911
relation summary #911
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #911 +/- ##
==========================================
- Coverage 90.74% 90.55% -0.20%
==========================================
Files 94 94
Lines 13549 13578 +29
Branches 13549 13578 +29
==========================================
Hits 12295 12295
- Misses 1140 1169 +29
Partials 114 114 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2
.
Benchmark suite | Current: 1400ec6 | Previous: cd8b37b | Ratio |
---|---|---|---|
merkle throughput/simd merkle |
27658225 ns/iter (± 670855 ) |
13712527 ns/iter (± 579195 ) |
2.02 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @shaharsamocha7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ohad-starkware)
crates/prover/src/constraint_framework/relation_tracker.rs
line 192 at r1 (raw file):
} type RelationInfo = (String, Vec<(Vec<M31>, M31)>);
Is there a non-destructive way to unify this, RelationEntry
and RelationTrackerEntry
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Alon-Ti)
crates/prover/src/constraint_framework/relation_tracker.rs
line 192 at r1 (raw file):
Previously, Alon-Ti wrote…
Is there a non-destructive way to unify this,
RelationEntry
andRelationTrackerEntry
?
the relation tracker could sit with the logup stuff under some "lookup_utils" folder in the constraint(air) framework
4816a4a
to
1400ec6
Compare
Previously, ohad-starkware (Ohad) wrote…
oh if you meant unifying to a single struct then no, this represents multiple entries that are not even from the same trace |
No description provided.